A printingui toolkit has native print dialog code[1]. I think we can move this code to widget and we can share common printingui implementation for all platform[2]. [1] http://searchfox.org/mozilla-central/rev/298033405057ca7aa5099153797467eceeaa08b5/toolkit/components/printingui/win/nsPrintingPromptService.cpp [2] http://searchfox.org/mozilla-central/source/toolkit/components/printingui

Comment on attachment 8914621 [details] [diff] [review] [WIP]move-native-dialog This patch will change the SetWindowText() and CreateWindow() to SetWindowTextW() and CreateWindowW() in order to treat localized string correctly as well. We can reproduce this localized string bug if print any page with <frame> tag on windows environment. [1] http://searchfox.org/mozilla-central/rev/a4702203522745baff21e519035b6c946b7d710d/toolkit/components/printingui/win/nsPrintDialogUtil.cpp#126-127

Comment on attachment 8914642 [details] Bug 1405210 - Part 1: Add PrintDialogService to windows widget. https://reviewboard.mozilla.org/r/185966/#review195450 ::: widget/moz.build (Diff revision 1) > toolkit = CONFIG['MOZ_WIDGET_TOOLKIT'] > > if toolkit in ('cocoa', 'android', 'uikit'): > DIRS += [toolkit] > -if toolkit in ('android', 'cocoa', 'gtk2', 'gtk3'): > - EXPORTS += ['nsIPrintDialogService.h'] note build changes will require a build config peer r+. ::: widget/windows/nsWindowBase.cpp:14 (Diff revision 1) > #include "mozilla/MiscEvents.h" > #include "KeyboardLayout.h" > #include "WinUtils.h" > #include "npapi.h" > #include "nsAutoPtr.h" > +#include "nsIPresShell.h" did we need this?

Comment on attachment 8914643 [details] Bug 1405210 - Part 2: Move native printing dialog code to windows widget. https://reviewboard.mozilla.org/r/185968/#review195452 ::: widget/windows/nsPrintDialogWin.cpp:31 (Diff revision 1) > > using namespace mozilla; > +using namespace mozilla::widget; > + > +/**************************************************************** > + ************************* ParamBlock *************************** slim this big ugly header comment down plz. ::: widget/windows/nsPrintDialogWin.cpp:37 (Diff revision 1) > + ****************************************************************/ > + > +class ParamBlock { > + > +public: > + ParamBlock() nit - spacing is off ::: widget/windows/nsPrintDialogWin.cpp:95 (Diff revision 1) > + NS_ENSURE_ARG(aNSSettings); > + > + ParamBlock block; > + nsresult rv = block.Init(); > + if (NS_FAILED(rv)) > + return rv; nit - brace me ::: widget/windows/nsPrintDialogWin.cpp:103 (Diff revision 1) > + rv = DoDialog(aParent, block, aNSSettings, kPageSetupDialogURL); > + > + // if aWebBrowserPrint is not null then we are printing > + // so we want to pass back NS_ERROR_ABORT on cancel > + if (NS_SUCCEEDED(rv)) > + { nit - move '{' up one line ::: widget/windows/nsPrintDialogWin.cpp:131 (Diff revision 1) > + // (though we'd rather this didn't fail, it's OK if it does. so there's > + // no failure or null check.) > + // retain ownership for method lifetime > + nsCOMPtr<mozIDOMWindowProxy> activeParent; > + if (!aParent) > + { nit - move '{' up

Comment on attachment 8914644 [details] Bug 1405210 - Part 3: Apply clang-format to added print dialog widget code. https://reviewboard.mozilla.org/r/185970/#review195454

Comment on attachment 8914642 [details] Bug 1405210 - Part 1: Add PrintDialogService to windows widget. https://reviewboard.mozilla.org/r/185966/#review195450 Thank you so much for reviewing. > note build changes will require a build config peer r+. OK. ted, Could you please review this spart? > did we need this? This will prevent to unified build failure.

(In reply to Mantaroh Yoshinaga[:mantaroh] from comment #14) > Comment on attachment 8914642 [details] > Bug 1405210 - Part 1: Add PrintDialogService to windows widget. > > https://reviewboard.mozilla.org/r/185966/#review195450 > > Thank you so much for reviewing. > > > note build changes will require a build config peer r+. > > OK. > > ted, > Could you please review this spart? Oh, sorry. This r? is my mistake. mshal, Could you please confirm this part?

Comment on attachment 8914642 [details] Bug 1405210 - Part 1: Add PrintDialogService to windows widget. https://reviewboard.mozilla.org/r/185966/#review196512 Build changes LGTM!

Comment on attachment 8914642 [details] Bug 1405210 - Part 1: Add PrintDialogService to windows widget. https://reviewboard.mozilla.org/r/185966/#review196512 Thank you!

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9456f71428369853d6ef0bd139a7c55141209142

Pushed by mantaroh@gmail.com: https://hg.mozilla.org/integration/autoland/rev/64beab4cf557 Part 1: Add PrintDialogService to windows widget. r=jimm,mshal https://hg.mozilla.org/integration/autoland/rev/bbc8f61facef Part 2: Move native printing dialog code to windows widget. r=jimm https://hg.mozilla.org/integration/autoland/rev/fbdb84f3a8c4 Part 3: Apply clang-format to added print dialog widget code. r=jimm

https://hg.mozilla.org/mozilla-central/rev/64beab4cf557 https://hg.mozilla.org/mozilla-central/rev/bbc8f61facef https://hg.mozilla.org/mozilla-central/rev/fbdb84f3a8c4